Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LLVM][TableGen] Change DisassemblerEmitter to use const RecordKeeper #109177

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Sep 18, 2024

Change DisassemblerEmitter to use const RecordKeeper.

This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089

@jurahul jurahul marked this pull request as ready for review September 18, 2024 22:29
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 20, 2024

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

Change DisassemblerEmitter to use const RecordKeeper.

This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089


Full diff: https://github.com/llvm/llvm-project/pull/109177.diff

3 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+4-8)
  • (modified) llvm/utils/TableGen/DisassemblerEmitter.cpp (+6-8)
  • (modified) llvm/utils/TableGen/TableGenBackends.h (+2-2)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index edecb9067bccf7..9cae7ec838d2a5 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -159,7 +159,7 @@ class DecoderEmitter {
   std::vector<EncodingAndInst> NumberedEncodings;
 
 public:
-  DecoderEmitter(const RecordKeeper &R, const std::string &PredicateNamespace)
+  DecoderEmitter(const RecordKeeper &R, StringRef PredicateNamespace)
       : RK(R), Target(R), PredicateNamespace(PredicateNamespace) {}
 
   // Emit the decoder state machine table.
@@ -181,7 +181,7 @@ class DecoderEmitter {
   CodeGenTarget Target;
 
 public:
-  const std::string &PredicateNamespace;
+  StringRef PredicateNamespace;
 };
 
 } // end anonymous namespace
@@ -2651,11 +2651,7 @@ namespace llvm {
   OS << "\n} // end namespace llvm\n";
 }
 
-namespace llvm {
-
-void EmitDecoder(RecordKeeper &RK, raw_ostream &OS,
-                 const std::string &PredicateNamespace) {
+void llvm::EmitDecoder(const RecordKeeper &RK, raw_ostream &OS,
+                       StringRef PredicateNamespace) {
   DecoderEmitter(RK, PredicateNamespace).run(OS);
 }
-
-} // end namespace llvm
diff --git a/llvm/utils/TableGen/DisassemblerEmitter.cpp b/llvm/utils/TableGen/DisassemblerEmitter.cpp
index f2c25d38ad2a7d..eb15392272a3f3 100644
--- a/llvm/utils/TableGen/DisassemblerEmitter.cpp
+++ b/llvm/utils/TableGen/DisassemblerEmitter.cpp
@@ -95,19 +95,17 @@ using namespace llvm::X86Disassembler;
 /// X86RecognizableInstr.cpp contains the implementation for a single
 ///   instruction.
 
-static void EmitDisassembler(RecordKeeper &Records, raw_ostream &OS) {
-  CodeGenTarget Target(Records);
+static void EmitDisassembler(const RecordKeeper &Records, raw_ostream &OS) {
+  const CodeGenTarget Target(Records);
   emitSourceFileHeader(" * " + Target.getName().str() + " Disassembler", OS);
 
   // X86 uses a custom disassembler.
   if (Target.getName() == "X86") {
     DisassemblerTables Tables;
 
-    ArrayRef<const CodeGenInstruction *> numberedInstructions =
-        Target.getInstructionsByEnumValue();
-
-    for (unsigned i = 0, e = numberedInstructions.size(); i != e; ++i)
-      RecognizableInstr::processInstr(Tables, *numberedInstructions[i], i);
+    for (const auto &[Idx, NumberedInst] :
+         enumerate(Target.getInstructionsByEnumValue()))
+      RecognizableInstr::processInstr(Tables, *NumberedInst, Idx);
 
     if (Tables.hasConflicts()) {
       PrintError(Target.getTargetRecord()->getLoc(), "Primary decode conflict");
@@ -126,7 +124,7 @@ static void EmitDisassembler(RecordKeeper &Records, raw_ostream &OS) {
     return;
   }
 
-  std::string PredicateNamespace = std::string(Target.getName());
+  StringRef PredicateNamespace = Target.getName();
   if (PredicateNamespace == "Thumb")
     PredicateNamespace = "ARM";
   EmitDecoder(Records, OS, PredicateNamespace);
diff --git a/llvm/utils/TableGen/TableGenBackends.h b/llvm/utils/TableGen/TableGenBackends.h
index fc3b87370766a3..d1cbc5d1605d8c 100644
--- a/llvm/utils/TableGen/TableGenBackends.h
+++ b/llvm/utils/TableGen/TableGenBackends.h
@@ -64,8 +64,8 @@ class RecordKeeper;
 void EmitMapTable(const RecordKeeper &RK, raw_ostream &OS);
 
 // Defined in DecoderEmitter.cpp
-void EmitDecoder(RecordKeeper &RK, raw_ostream &OS,
-                 const std::string &PredicateNamespace);
+void EmitDecoder(const RecordKeeper &RK, raw_ostream &OS,
+                 StringRef PredicateNamespace);
 
 } // namespace llvm
 

@jurahul jurahul merged commit b594b93 into llvm:main Sep 20, 2024
12 checks passed
@jurahul jurahul deleted the const_record_disassembler_emitter branch September 20, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants